src/mesh/comms/FakeUART.h:24

• The flush(bool) signature doesn’t match the Stream::flush() override (which takes no parameters), potentially leading to unexpected behavior; consider renaming or correctly overriding void flush().

    void flush(bool wait = true);

src/mesh/comms/FakeI2C.cpp:37

• Returning 0xFF when quantity != 1 may be misinterpreted as 255 bytes available; return 0 to match the Wire API's convention of 0 bytes read on failure.

  if (quantity != 1) return 0xFF;

src/RedirectablePrint.cpp (2)

81-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Comment exceeds the 1–2 line limit.

This explanatory comment spans three lines. As per coding guidelines, "Keep code comments minimal: one or two lines maximum, only when the reason is not obvious, and do not restate the next line."

✏️ Suggested trim

-    // A leading "[Tag] " in the message simulates the thread name tag (used
-    // by contexts without an OSThread, e.g. the device-ui task). Print it
-    // before applying the level color so it renders like a real thread name.
+    // A leading "[Tag] " simulates a thread-name tag (e.g. for device-ui);
+    // print it uncolored, before the level color codes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/RedirectablePrint.cpp` around lines 81 - 83, Shorten the explanatory
comment near the level-color printing logic to no more than two lines while
preserving only the essential reason for printing the leading “[Tag] ” before
applying the color. Remove redundant wording that restates the surrounding code.

Source: Coding guidelines

---

85-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Tag-detection window is a magic number.

The hardcoded 24 bound for the tag scan window has no named constant or brief rationale. Consider extracting it (e.g. constexpr size_t kMaxTagScanLen = 24;) for readability.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/RedirectablePrint.cpp` around lines 85 - 89, Extract the hardcoded
24-byte scan bound in the tag-detection logic around printBuf into a named
constexpr constant such as kMaxTagScanLen, and use that symbol in the memchr
length expression while preserving the existing limit.

src/mesh/IndicatorSerial.cpp (2)

256-258: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten this implementation comment.

It exceeds the two-line limit and largely describes the following operations.

As per coding guidelines, “Keep code comments minimal: one or two lines maximum.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/IndicatorSerial.cpp` around lines 256 - 258, Shorten the comment
above the protobuf decoding logic to one or two concise lines, retaining only
that decoding occurs and any remaining buffer bytes are preserved for the next
packet.

Source: Coding guidelines

---

47-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use Throttle for the timeout checks.

Both loops implement elapsed-time checks with raw millis() subtraction.

As per coding guidelines, use Throttle instead of raw millis() arithmetic for “did N ms pass” checks.

Also applies to: 173-185

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/IndicatorSerial.cpp` around lines 47 - 58, Update
SensecapIndicator::wait_response and the other referenced response-wait loop to
use the existing Throttle mechanism for elapsed-time checks instead of millis()
subtraction. Configure Throttle with the requested timeout and use its
expiration/check API while preserving the current flag handling, pumping, delay,
and timeout return behavior.

Source: Coding guidelines

src/mesh/IndicatorSerial.h (1)

33-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep implementation comments to two lines or fewer.

These newly added comments exceed the repository limit; condense each without restating implementation details.

As per coding guidelines, “Keep code comments minimal: one or two lines maximum.”

Also applies to: 38-40, 54-56, 71-74

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/IndicatorSerial.h` around lines 33 - 35, Condense the newly added
comments in IndicatorSerial around the request/response flow and the
corresponding sections at the other referenced locations so each is no more than
two lines. Keep only the essential intent, removing redundant implementation
details and preserving the surrounding code unchanged.

Source: Coding guidelines

src/mesh/comms/FakeUART.cpp (1)

58-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

setRxBufferSize reports success without actually resizing anything.

Returning size unconditionally (regardless of the fixed BUF_SIZE = 2048 buffer) can mislead callers expecting the requested capacity was honored. Consider returning the actual BUF_SIZE instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/comms/FakeUART.cpp` around lines 58 - 61, Update
FakeUART::setRxBufferSize to return the actual fixed receive-buffer capacity,
BUF_SIZE, instead of the requested size; leave the existing buffer behavior
unchanged.

src/mesh/comms/FakeI2C.cpp (1)

44-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Silent clamp of requestFrom length instead of an invariant check.

len is silently truncated to MAX_READ (line 47-48) rather than treated as a caller bug. MAX_READ is derived from the negotiated protobuf limit (line 50-51 in the header), so any caller requesting more indicates a driver bug, not a runtime condition to gracefully clamp.

🛡️ Suggested change

 size_t FakeI2C::requestFrom(uint8_t address, size_t len, bool stopBit)
 {
     (void)stopBit;
-    if (len > MAX_READ)
-        len = MAX_READ;
+    assert(len <= MAX_READ);

As per coding guidelines, "Use assert() for invariants that should never fail."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/comms/FakeI2C.cpp` around lines 44 - 60, Replace the silent len
truncation in FakeI2C::requestFrom with an assert that len does not exceed
MAX_READ, preserving the existing transaction flow for valid requests. Include
the appropriate assert support if needed and remove the clamp assignment.

Source: Coding guidelines

src/mesh/comms/FakeI2C.h (1)

8-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Class doc comment exceeds the 1–2 line limit.

This 15-line block comment explains multiple behaviors (buffering semantics, repeated-start combining, bus-0 construction rationale) in one long explanatory block.

✏️ Suggested trim

-/**
- * TwoWire implementation that tunnels bus traffic to the RP2040 over the
- * interdevice serial link, making the sensors attached to the secondary MCU
- * usable by the regular sensor drivers of the main firmware.
- *
- * beginTransmission()/write() buffer an outgoing write. The buffered write is
- * executed as a single serial round trip on endTransmission(true). A write
- * followed by endTransmission(false) and requestFrom() is combined into one
- * write+read transaction with repeated start, matching driver access
- * patterns.
- *
- * Constructed on bus number 0: TwoWire::begin() is final and cannot be
- * intercepted, but it returns without touching hardware when the bus is
- * already initialized, which is always true for bus 0 (local touch panel).
- */
+// Tunnels TwoWire traffic over the interdevice serial link to the RP2040.
+// write()+endTransmission(false)+requestFrom() combine into one repeated-start transaction.

As per coding guidelines, "Keep code comments minimal: one or two lines maximum, only when the reason is not obvious, and do not restate the next line."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/comms/FakeI2C.h` around lines 8 - 22, Shorten the class-level
documentation for TwoWire to one or two lines, retaining only the non-obvious
purpose or rationale. Remove the detailed buffering, transaction, and bus-0
behavior explanation from the comment without changing the implementation.

Source: Coding guidelines

src/mesh/comms/FakeUART.h (1)

23-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

flush(bool wait = true) also hides rather than overrides Print::flush().

Same root cause as the write() mismatch: Print::flush() takes no arguments, so this signature hides it instead of overriding. Likely intentional to mirror HardwareSerial::flush(bool), but worth confirming it's never called through a Stream&/Print& reference where the argument would be silently dropped.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/comms/FakeUART.h` at line 23, Update FakeUART::flush so it does not
hide Print::flush(): preserve the bool-based HardwareSerial-compatible API while
also exposing a no-argument flush path that correctly overrides Print::flush().
Ensure calls through Stream& or Print& invoke the no-argument override, and
retain the existing wait behavior for direct bool-based calls.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/RedirectablePrint.cpp (1)

81-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the explanatory comment within the two-line limit.

This new comment spans three lines; condense it to one or two lines while preserving only the non-obvious rationale.

As per coding guidelines, C++ comments must be at most one or two lines and must not restate the following code.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/RedirectablePrint.cpp` around lines 81 - 83, Condense the explanatory
comment above the leading “[Tag] ” handling to no more than two lines, retaining
only the non-obvious rationale that it simulates a thread name tag for contexts
without an OSThread and renders it like a real thread name. Remove wording that
restates the following code.

Source: Coding guidelines

src/mesh/IndicatorSerial.cpp (2)

365-371: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

memmove always shifts a full-buffer-sized region instead of the actual remaining bytes.

The length passed is PB_BUFSIZE - MT_HEADER_SIZE - payload_len, computed from the buffer capacity rather than pb_rx_size. Every processed frame (pings, NMEA, i2c results, file chunks) shifts up to the full remaining buffer capacity instead of only the valid bytes, wasting cycles on this hot path at 2M baud.

⚡ Proposed fix

-    pb_istream_t stream = pb_istream_from_buffer(pb_rx_buf + MT_HEADER_SIZE, payload_len);
-    bool status = pb_decode(&stream, meshtastic_InterdeviceMessage_fields, &message);
-    memmove(pb_rx_buf, pb_rx_buf + MT_HEADER_SIZE + payload_len, PB_BUFSIZE - MT_HEADER_SIZE - payload_len);
-    pb_rx_size -= MT_HEADER_SIZE + payload_len;
+    pb_istream_t stream = pb_istream_from_buffer(pb_rx_buf + MT_HEADER_SIZE, payload_len);
+    bool status = pb_decode(&stream, meshtastic_InterdeviceMessage_fields, &message);
+    size_t remaining = pb_rx_size - MT_HEADER_SIZE - payload_len;
+    memmove(pb_rx_buf, pb_rx_buf + MT_HEADER_SIZE + payload_len, remaining);
+    pb_rx_size = remaining;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/IndicatorSerial.cpp` around lines 365 - 371, Update the memmove call
in the protobuf decode path to copy only the valid bytes remaining after the
processed frame, using pb_rx_size minus MT_HEADER_SIZE and payload_len as the
length. Preserve the existing source and destination offsets and subsequent
pb_rx_size adjustment.

---

42-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use Throttle instead of raw millis() arithmetic.

probe_link()'s 250ms cooldown (Line 44) and the deadline checks in wait_response() (Line 74) and wait_ready() (Line 278) are exactly the "did N ms pass since X" pattern the guideline calls out for Throttle.

♻️ Proposed fix for probe_link

-    if (last_probe != 0 && millis() - last_probe < 250)
-        return;
+    if (last_probe != 0 && Throttle::isWithinTimespanMs(last_probe, 250))
+        return;

The same substitution applies to the timeout checks at Line 74 and Line 278.

Also applies to: 65-79, 261-283

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/IndicatorSerial.cpp` around lines 42 - 53, Replace raw
millis()-based elapsed-time checks in SensecapIndicator::probe_link,
wait_response, and wait_ready with the established Throttle utility. Preserve
the current 250 ms probe cooldown and the existing response/readiness timeout
durations, using Throttle to track and test each operation’s deadline without
changing surrounding message or state behavior.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes